Repository navigation
[core] Negate predicates like Java so NULL rows stay excluded - #1145
JeonDaehong wants to merge 3 commits into
Conversation
|
@JingsongLi @leaves12138 could you take a look when you have time? Fixes #1143: |
|
Moved to draft until CI is green. I couldn't run the docker fixtures and the Go/Python suites locally. Will mark it ready once they pass. |
|
CI is green, ready for review. |
JingsongLi
left a comment
There was a problem hiding this comment.
Requirement fit: SUPPORTED. Implementation: FINDINGS at 0a5bdd9. Fixing NULL rows being selected/deleted by negated predicates has clear end-to-end value. The ordinary NULL and added string-negation scenarios pass, but two additional public-API regression tests fail on this head and pass on base 9568f39: floating bitmap + vector Top-1 returns a row that violates the filter, and a NaN partition is silently excluded. Details are inline. Local verification also ran the core library suite (4146 passed, 6 ignored); after provisioning the actual Spark warehouse, all 358 DataFusion library tests passed. Both new regression tests were also run unchanged against the baseline worktree and passed there.
| Some(match self { | ||
| Self::IsNull => Self::IsNotNull, | ||
| Self::IsNotNull => Self::IsNull, | ||
| Self::Eq => Self::NotEq, |
There was a problem hiding this comment.
[P1] Refine floating bitmap candidates before ranking normalized NOT filters
Turning NOT(f = 1) into NotEq now enters the floating bitmap scalar-index path. BitmapGlobalIndexReader::query deliberately returns all non-null rows for floating NotEq, but matching_ranges_with_exactness labels this leaf exact, so scalar_matched_row_ids skips refinement before vector ranking. I reproduced this through public write/commit/index/search APIs: two rows have f=[1,2], vectors [[1,0],[0,1]], a bitmap index on f, and an ivf-flat index on the vectors. Searching [1,0] with limit 1 and Predicate::negate(equal(f,1)) returns row 0 (f=1), which violates the filter; expected row 1. The same test passes on base 9568f39a, and explicitly constructing raw Predicate::Not still returns row 1 on this head. Propagate floating bitmap candidate inexactness to the scanner and refine before ranking; add this filtered Top-K regression test.
There was a problem hiding this comment.
Thanks, reproduced. Fixed in eff224e in two places:
negatenow keepsNotfor FLOAT/DOUBLE comparisons (see the NaN comment), soNOT(f = 1)stays out of the bitmapNotEqpath.- The same wrong Top-1 also came from a plain
not_equal(f, 1)orgreater_than(f, 1.5), with no negation involved.is_exact_predicatenow treats FLOAT/DOUBLE leaves that the bitmap answers with every non-null row (is_bitmap_floating_residual_sensitive_op) as inexact, so they are refined before ranking.
floating_bitmap_candidates_are_refined_before_top_k covers all three filters. Before the change it returned row 0, now row 1.
| Self::IsNotNull => Self::IsNull, | ||
| Self::Eq => Self::NotEq, | ||
| Self::NotEq => Self::Eq, | ||
| Self::Lt => Self::GtEq, |
There was a problem hiding this comment.
[P2] Preserve NaN rows when negating floating partition comparisons
These operator flips assume complementary ordering, but native datum_cmp uses partial_cmp for FLOAT/DOUBLE, so NaN makes both < and >= false. I reproduced a scan with rows (id,f)=[(0,0.0),(1,NaN),(2,2.0)] and Predicate::negate(less_than(f,1.0)): this head returns [1,2] when f is a data column but only [2] when partitioned by f, because the normalized GtEq prunes the NaN partition. The identical test returns [1,2] in both layouts on base 9568f39a. Java's floating comparison and the Arrow residual path retain the NaN row here. Make native floating comparison compatible with those semantics, or retain NULL-aware NOT until the complements are sound, and cover partitioned/nonpartitioned NaN reads.
There was a problem hiding this comment.
Thanks, reproduced: the partitioned read returned [2]. I kept Not for FLOAT/DOUBLE comparisons in negate rather than changing datum_cmp, since that would also change plain < / > on NaN partitions, which seems like a separate change. Not(leaf) is false only for NULL, and NaN < 1.0 is false, so the NaN row is kept, as in Java.
negated_float_comparison_keeps_nan_rows checks the read and delete_by_predicate, partitioned and not.
0a5bdd9 to
eff224e
Compare
|
@JingsongLi thanks for the careful review. Both are fixed in eff224e, details inline. I also rebased onto main. After #1153, Checked with the ci.yml steps: fmt, clippy ( |
JingsongLi
left a comment
There was a problem hiding this comment.
Requirement fit: SUPPORTED. Implementation: FINDINGS at eff224e. The prior floating-bitmap Top-1 and NaN-partition negation regressions are fixed on their covered paths; both added regression tests pass. A new public-API BTree + vector-search probe now returns no rows on this head and passes unchanged on baseline 46e6871. Details inline.
Local validation: 4,250 core library tests, 14 native predicate read/delete tests, 358 DataFusion library tests, 46 SQL read tests, and 5 C API checks passed. The actual Spark warehouse was provisioned. Two additional SQL reads fail only because liblumina_py.dylib is unavailable; lumina-data 0.1.0 supplies only a Linux x86_64 wheel, so these two native-library checks could not be completed on this macOS ARM host. Formatting and clean integration with current main were also checked.
| && !(matches!(data_type, DataType::Float(_) | DataType::Double(_)) | ||
| && super::bitmap_global_index_format::is_bitmap_floating_residual_sensitive_op(*op)) |
There was a problem hiding this comment.
[P2] Keep exact floating BTree answers exact
This bitmap-specific downgrade currently applies to BTree results as well. BTree already implements floating != and ranges using the same bit-preserving total ordering as the residual filter, so those answers are exact. With the supported global-index.filter.refine-from-data=false option, scalar_matched_row_ids excludes all candidates that this method marks inexact. I reproduced the regression using actual Parquet data with finite f=[1.0, 2.0], a scalar BTree on f, an ivf-flat vector index, and filtered Top-1 with f != 1.0: baseline 46e6871 returns id 1, while this head returns no rows. No NaN or predicate negation is involved. Please derive exactness from the contributing index entries/readers and downgrade the floating bitmap fallback specifically, retaining exact BTree results; add a regression with refinement disabled.
There was a problem hiding this comment.
Right, the downgrade was keyed on the predicate, not on the index that answered it. Fixed in ae3660e. evaluate_leaf now marks a leaf inexact only when a bitmap entry answered a floating range or negation, so BTree results stay exact. I reverted the check in is_exact_predicate.
floating_bitmap_candidates_are_refined_before_top_k now also runs on BTree, with and without refine-from-data=false. Before this change, BTree + refine-from-data=false returned [].
I also rebased onto current main.
Predicate::negate wrapped leaves in Not, which was evaluated as !inner, so NOT(p = 'a') selected rows where p is NULL. Reads returned them and delete_by_predicate deleted them. Flip the leaf operator and push through AND/OR like Java Predicate.negate. Leaves without a negated operator stay as Not, which now evaluates to false for NULL input. Closes apache#1143
…ates Negating a FLOAT/DOUBLE comparison keeps Not, since NaN makes the flipped operator non-complementary. Scalar index exactness treats FLOAT/DOUBLE leaves that the bitmap index answers with every non-null row as inexact, so filtered Top-K refines them before ranking.
Only a bitmap answers floating ranges and negations with every non-null row, so mark a leaf inexact only when a bitmap entry answered it.
eff224e to
ae3660e
Compare
JingsongLi
left a comment
There was a problem hiding this comment.
Requirement fit: SUPPORTED. Implementation: FINDINGS at ae3660e. Fixing negated predicates selecting/deleting NULL rows has clear end-to-end value. The earlier bitmap Top-K, NaN-partition and pure-BTree regressions are covered and their tests pass, but two additional public-API regressions remain: a pruned all-NULL Bitmap shard downgrades exact BTree hits, and raw double-NOT selects NULL after schema evolution. Both tests fail on this head and pass unchanged on exact base cea0caa; details inline.
Local validation: 4,449 core library tests (7 ignored), 358 DataFusion library tests, 14 native predicate read/delete tests and 46 SQL read tests passed. Two other SQL reads could not run successfully because this macOS host lacks liblumina_py.dylib. The current CI check failure is the unchanged row_variant_interop README license header.
| // A bitmap answers floating ranges and negations with every non-null | ||
| // row (see `bitmap_meta_may_match`), so those results are candidates | ||
| // only. Sorted indexes answer them exactly. | ||
| let bitmap_superset = entries |
There was a problem hiding this comment.
[P2] Preserve exactness when every Bitmap entry was pruned
This flag considers every selected entry before file-level pruning. I reproduced a public write/index/search scenario that builds Bitmap(f) over two NULL rows, appends finite f=[2,3], then builds BTree(f) and an IVF-flat vector index. For f > 1.5, the Bitmap shard is pruned by its all-NULL metadata and the remaining BTree answer is exact. Nevertheless bitmap_superset downgrades it, so Top-1 with global-index.filter.refine-from-data=false returns [] instead of row 2. The same test passes unchanged on base cea0caa; refinement=true also returns row 2 on this head. Derive candidate inexactness from Bitmap entries that actually contribute a superset, retaining exactness when they are all pruned.
| }))) | ||
| let negated = | ||
| boolean_mask_from_predicate(mask.len(), |row_index| !mask.value(row_index)); | ||
| if let Some(guard) = inner.not_null_guard() { |
There was a problem hiding this comment.
[P2] Keep raw double negation from selecting NULL rows
The new guard makes an inner Not(leaf) false for NULL, but an outer Not has no guard when its child is another Not, so it turns that false back into true. Public Predicate::Not is a logical-negation variant and existing code accepts raw nested Not trees. A public read probe writes value=[NULL,1,2], persists a supported INT-to-BIGINT schema evolution, and reads with Not(Not(value = 1)): this head returns ids [0,1], while exact base cea0caa returns only [1]. This is a newly introduced double-NOT regression, separate from the pre-existing raw Not(AND/OR) limitation. Normalize/eliminate double negation before evaluation, or preserve NULL semantics through nested Not consistently in both residual and BinaryRow evaluation.
Purpose
Linked issue: close #1143
Predicate::negatewrapped leaves inNot, andNotwas evaluated as!inner, so a NULL input madeNOT(p = 'a')true. Reads returned those rows anddelete_by_predicatedeleted them. Java flips the leaf instead (Equal.negate()isNotEqual), which is false for NULL.Brief change log
Predicate::negatenow does what JavaPredicate.negatedoes: flip the leaf operator (=/!=,</>=,<=/>,IS NULL/IS NOT NULL,IN/NOT IN,BETWEEN/NOT BETWEEN) and push through AND/OR with De Morgan.scan.rsnow matches the new shape of the negated filter (x < lo OR x > hiinstead ofNOT(x >= lo AND x <= hi)).STARTS_WITH,ENDS_WITH,CONTAINS,LIKE, array ops) stay asNot(leaf).eval_rowand the residual row filter now return false for those when the input is NULL, same as JavaNotStartsWith/NotLike/... .Tests
negated_predicates_keep_null_rowsandnegated_data_predicate_keeps_null_rowscover the cases from the issue on the partition and DV delete paths and on reads. Both fail on main.negateand for NULL evaluation ineval_rowand the residual filter.test_file_index_nested_not_with_added_column_falls_backnow expects[1]instead of[1, 2].addedis NULL in the old file, so for id 2 the predicate isNOT(true AND unknown). Java drops that row too.test_not/test_display_not, three DataFusion pushdown tests, the global index exactness test and the stats fail-open test asserted the oldNotshape. The last two now buildPredicate::Notdirectly so they keep testing theNotbranch.fmt and clippy (1.98.0,
-D warnings) pass.cargo test --no-fail-fast -p paimon -p paimon-datafusion -p paimon-c: 5804 passed. The 39 failures are allTableNotExist/ table not found for the fixture tables CI provisions, plus 2 that need the lumina library. The ones I checked fail the same way on main.Not covered here: a
Predicate::Notaround AND/OR built by hand (not throughnegate) is still evaluated as!inner. Nothing in core builds that today, but I can add it if you'd like.API and Format
No API change.
Predicate::negatereturns a different but equivalent tree for non-NULL inputs.Documentation
None.